Skip to content

sapi: remove reimplementations of (un)setenv - #23287

Open
NattyNarwhal wants to merge 1 commit into
php:masterfrom
NattyNarwhal:fpm-remove-wtf-setenv
Open

sapi: remove reimplementations of (un)setenv#23287
NattyNarwhal wants to merge 1 commit into
php:masterfrom
NattyNarwhal:fpm-remove-wtf-setenv

Conversation

@NattyNarwhal

Copy link
Copy Markdown
Member

These are functions that have existed since Unix V7. Windows doesn't have them, but we don't support FPM on Windows (and in ext/standard, there are other ways to emulate it that don't involve WTF comments).

clearenv is kept as this was from a rejected POSIX proposal that only some systems implement (Linux, FreeBSD, some 90s Unices).

Also removes the WTF comment incidentally; see GH-23285.

These are functions that have existed since Unix V7. Windows doesn't
have them, but we don't support FPM on Windows (and in ext/standard,
there are other ways to emulate it that don't involve WTF comments).

clearenv is kept as this was from a rejected POSIX proposal that only
some systems implement (Linux, FreeBSD, some 90s Unices).

Also removes the WTF comment incidentally; see phpGH-23285.
@NattyNarwhal
NattyNarwhal requested a review from bukka as a code owner August 14, 2026 21:52
@NattyNarwhal NattyNarwhal changed the title sapi/fpm: remove reimplementations of (un)setenv sapi: remove reimplementations of (un)setenv Aug 18, 2026
@NattyNarwhal
NattyNarwhal force-pushed the fpm-remove-wtf-setenv branch from b9dd926 to 742254f Compare August 18, 2026 19:32
@NattyNarwhal

Copy link
Copy Markdown
Member Author

FWIW, I had a commit to remove the setenv/unsetenv ifdefs on Windows per #23285 (comment) but removed it, Windows will need a bigger refactor as it only has putenv in CRT (and the Win32 SetEnvironmentVariable is more like putenv IIRC). I suspect it might be a good idea to separate out the environment handling code in ext/standard/basic_functions.c to its own file in main/.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant